fix(duckdb): normalize a timestamp operand only when its type needs it (fixes spiceai/spiceai#12574) - #50

Summary

The DuckDB comparison rewrite renders a bare operand as TO_TIMESTAMP(EPOCH_MS(<operand>) / 1000)
so DuckDB sees two TIMESTAMPTZ values. It decided that without knowing the operand's type,
because no schema reached the unparser — so it also fired on columns that compare exactly without
it, truncating them to whole milliseconds. A </>/= against a literal inside that millisecond
then selected different rows, silently.

That is not a corner case. Timestamp(_, Some(tz)) is what every timezone-aware column in a
DuckDB-accelerated dataset lands on, and it maps to TIMESTAMPTZ, the one type the rewrite could
only harm.

Measured against DuckDB v1.5.5 rather than reasoned about:

So the timezone decides it, not the unit, and only the last row changes behaviour.

The literal on the other side of that comparison was truncated too, by integer division:
2026-01-01 00:00:00.000999Z rendered as TO_TIMESTAMP(1767225600). An exact column compared
against a whole-second literal is still the wrong answer, so both halves are fixed here. A
whole-second literal renders byte-identically, which is why no existing expectation moved.

Changes

• expr.rs: to_sql_with_engine_and_schema takes the schema the expression's columns come from and
  threads it to the one site that needs it. to_sql_with_engine delegates with None, so a caller
  with no schema to offer renders exactly what it rendered before.
• expr.rs: duckdb_normalizes_timestamp_operand resolves the operand against that schema.
  Un-resolvable — no schema, not a Column, or a column the schema does not carry — normalizes, as
  before; only a resolved type declines. A non-temporal operand also stops being normalized: it
  never bound either way, but DuckDB now names the types it could not compare instead of failing on
  the division the rewrite introduced.
• expr.rs: the DuckDB timestamp literal arms keep their sub-second digits. The nanosecond arm
  drops to microseconds in integer arithmetic — 1.7e18 exceeds the 2^53 an f64 holds exactly, and
  a TIMESTAMPTZ cannot carry nanoseconds anyway.
• util/dml.rs: filters_to_sql_with_schema / assignments_to_sql_with_schema, with the existing
  two delegating to them.
• duckdb/write.rs: delete_from and update pass the table's schema.

Performance impact

None on any per-row or per-batch path. filters_to_sql* is reached only from
TableProvider::delete_from / update, which DataFusion calls once at plan time; the rendered
String is stored on the sink and the DELETE/UPDATE then executes entirely inside DuckDB. The
schema lookup is a linear scan over the field list, once per comparison, and Option<&Schema> is a
register-sized Copy through the recursion.

Test plan

• make lint
• make test

New tests, all executed against a real in-memory DuckDB rather than asserting on rendered text
alone:

• a_microsecond_timestamptz_deletes_the_row_inside_the_millisecond — the reported defect. Two rows
  a microsecond apart, a filter naming the instant between them; asserts the right row goes, and
  that with the column-side truncation left in place it does not.
• a_naive_millisecond_column_is_still_normalized_and_still_binds — the rewrite still covers what it
  was written for.
• subtracting_a_timestamp_from_a_timezone_aware_column_still_binds and
  test_duckdb_does_not_normalize_a_non_temporal_column — the two arms the type test newly leaves
  bare.
• an_update_stores_the_sub_second_instant_it_was_assigned — a SET value carries the same literal
  rendering as a filter.
• Rendering tests over all four TimeUnits with and without a timezone, plus the no-schema,
  absent-schema and column-not-in-schema fallbacks.

Both production changes were neutered independently and re-neutered after the review pass: forcing
the type test to always normalize fails 4 tests, restoring integer division on the literal fails 4.

Review gate

• Adversarial review: skipped — codex exited 1 with "Your workspace is out of credits. Ask
  your workspace owner to refill in order to continue."; grok is not installed, so the receipt
  records engine=none exit=1. Its angles were hand-run instead, and two changed the diff: whether
  a non-temporal operand could now bind rather than error (measured — it still refuses, with a
  better message), and whether TIMESTAMPTZ - TIMESTAMPTZ binds once subtraction is no longer
  normalized (measured — it does, returning the same interval). Both are now tests.
• /security-review: skipped — it scoped itself to the invoking session's working directory and
  was handed an empty diff, which would have minted a clean pass over nothing. Hand-audited instead.
  The only category that applies is SQL injection, and the one new rendering path formats an i64
  through f64 Display, which cannot emit a quote, whitespace, exponent, inf or NaN; the new
  schema lookup uses the column name as a lookup key only, and identifiers still render through
  quoted_identifier. The gate cannot widen a WHERE clause into a tautology — it decides whether
  an operand is wrapped, never the predicate's structure.
• /simplify: ran, 4 agents. Applied: collapsed the type test to Timestamp(_, None) (the unit
  arms all agreed); column_with_name for field_with_name, which built and discarded a Vec of
  every field name on a supported path; bound the scaled seconds once per literal arm and the schema
  once in update; a shared fixture and one query helper for the DuckDB tests. It also caught two
  comments this change had made false — is_duckdb_timestamp_operand claiming an operand's type is
  not visible, and handle_cast claiming its rendering must match the literal arms, which now
  differ in precision. Skipped, with reasons: changing the three existing signatures instead of
  adding *_with_schema (this crate is published upstream as datafusion-contrib/datafusion-table-providers,
  so additive is the merge-friendly shape, and to_sql → to_sql_with_engine is the same pattern
  already in this file); merging the new test module into duckdb_execution_tests (its helper is
  hardcoded to a different fixture, so it would mean rewriting tests outside this diff); a
  RenderCtx struct for the two context parameters (churns twelve call sites for no behaviour
  change).

Review round 2 — Copilot

Three findings, all valid, all acted on:

• A resolved Date32/Date64 column was being taken out of the normalization set. A real
  regression, and the one case where the rewrite is load-bearing for the reference frame rather
  than for binding: DuckDB promotes a bare DATE to a TIMESTAMPTZ at midnight in the session's
  TimeZone, while the rendered literal is midnight UTC. Measured on v1.5.5,
  "dt" = TO_TIMESTAMP(1767225600) answers false under America/Los_Angeles and true under
  UTC. Dates stay normalized; a DELETE run under both session timezones pins it.
• The doc called the rewrite lossless for naive timestamps. It is not — EPOCH_MS truncates a
  naive TIMESTAMP or TIMESTAMP_NS to the millisecond, measured. Corrected, with the remainder
  filed as A naive microsecond or nanosecond timestamp column is still truncated to the millisecond by the DuckDB normalization spiceai#13146 and the reason it is kept anyway (dropping it would trade a bounded
  truncation for a session-timezone-dependent result).
• The literal-precision comment overclaimed. Also correct: TO_TIMESTAMP takes a DOUBLE, so
  microsecond resolution holds only to about the year 2255 — Arrow's nanosecond range runs to 2262.
  Its suggested remedy, rendering the fraction from integer arithmetic, does not fix it: at 2260
  both forms land on the same wrong microsecond, because DuckDB's own parse is what rounds, and the
  naive {sec}.{frac:06} form is outright wrong for pre-epoch values (-1µs renders as -1.999999,
  which round-trips to -1999999µs). The rendering is unchanged and the comment now states the real
  bound.

Follow-ups filed

• Timestamp literals lose every digit below the second for ODBC, Spark and ClickHouse spiceai#13143 — the same second-truncation still applies to ODBC, Spark and ClickHouse,
  which share the catch-all arm. Left out because it needs measuring against each engine.
• The DuckDB search-index filter pushdown renders without a schema, so it still truncates a timezone-aware timestamp spiceai#13144 — the DuckDB search-index filter pushdown in spiceai/spiceai renders
  without a schema and has one in scope; blocked on the rev bump for this PR.
• A DuckDB comparison between two timestamp columns is never normalized, so DuckDB refuses the statement spiceai#13145 — a comparison between two timestamp columns is still never normalized, so
  DuckDB refuses it. Fixing that widens the rewrite where this change narrows it.
• A naive microsecond or nanosecond timestamp column is still truncated to the millisecond by the DuckDB normalization spiceai#13146 — a naive microsecond or nanosecond column is still truncated to the
  millisecond by the rewrite it keeps. Pinning UTC losslessly needs AT TIME ZONE, an ICU-extension
  dependency this rendering does not take.

Fixes spiceai/spiceai#12574